-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Modernize DemoFrame #20664
[docs] Modernize DemoFrame #20664
Conversation
c51f38d
to
3738008
Compare
Details of bundle changes.Comparing: 324f9c5...6423280 Details of page changes
|
|
||
// NoSsr fixes a strange concurrency issue with iframe and quick React mount/unmount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliviertassinari Do you recall the symptoms of that issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could find #13705 as the origin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to make sure https://deploy-preview-20664--material-ui.netlify.app/zh/components/drawers/ displays without a crash. It was before hooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check locally if /zh/components/drawers/
crashes. That was the issue in #13705, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not crash locally in small and large viewporet.
I got warnings in concurrent mode for "state transition during render" which is indicative of some issue with react-frame-component. Might just be an issue with class components and concurrent mode.
Hopefully this fixes concurrent mode issues as well. If not then at least let it not be a bug in react :)
This component causes some issues in concurrent mode. I used the opportunity to simplify the implementation:
remove state.ready and instead derive it directlyuse a ref to the insertion point div instead ofwindow['jss-insertion-point']
. That's a cute alternative towindow.document.querySelector('#jss-insertion-point')
but, let's be honest, it's just showing of knowledge of hidden browser APIs 😉container
prop. We only have a single usage for it and can be replace with a ref-based approach which isolates the demo better.Tested on IE11 (win10) and Safari 13.0 (macOS 10.15).